Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor: upgrade ethers.js v6 #454

Merged
merged 9 commits into from
Apr 12, 2024
Merged

refactor: upgrade ethers.js v6 #454

merged 9 commits into from
Apr 12, 2024

Conversation

neel-ds
Copy link
Contributor

@neel-ds neel-ds commented Mar 27, 2024

Description

Upgraded the ethers.js dependency to v6 across most apps. To support this upgrade, the dependency was also upgraded in the libs. Related dependencies like Wagmi, Rainbowkit, and Typescript were updated for compatibility. Currently experimenting with downgrading the typescript version to resolve remaining union-type errors.

Related Issue

This PR resolves the issue #366

Does this introduce a breaking change?

  • Yes
  • No

Other information

  • The upgrade was necessary to keep the project up-to-date with the latest ethers.js version and take advantage of new features and bug fixes.
  • Downgrading the typescript version is an experimental approach to resolve the remaining union-type errors, which may or may not work. You can find the build error here
  • If the typescript version downgrade doesn't resolve the errors, further investigation and changes may be required.

@vercel Vercel
Copy link

vercel bot commented Mar 27, 2024 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
bandada-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 11, 2024 6:27am

@vplasencia vplasencia requested review from 0xjei and vplasencia March 28, 2024 10:15
0xjei
0xjei suggested changes Mar 28, 2024
View reviewed changes
Copy link
Contributor

@0xjei 0xjei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @neel-ds for your patience and effort. I have tried everything locally and the build still fails. By the way, I have managed the next steps to successfully complete the update. You can follow this list.

  1. Add // @ts-ignore for getting rid of TS2590: Expression produces a union type that is too complex to represent. You can add the line before the tags in the line that the build error indicates to you. We can open an issue and tackle this error later since is non-blocking.
  2. Update vite and @vitejs/plugin-react to the latest versions on the dashboard app.
  3. Downgrade chai on contracts package to 4.3.7 since >= v5 is ESM only.
  4. Upodate @typechain/hardhat and @nomicfoundation/hardhat-ethers to latest version on contracts app
  5. Add the following missing dev dependency on contracts app
  • @nomicfoundation/hardhat-ignition
  • @nomicfoundation/hardhat-ignition-ethers
  • @nomicfoundation/ignition-core
  1. On the dashboard app index.html add the following script below the <body> tag. 
    <script>
    var global = global || window
</script>
  2. Update siwe package to latest on api and dashboard apps.
  3. Add missing Node polyfills to dashaboard app.
    • Install vite-plugin-node-polyfills as dev dependency
    • Add them on vite.config.ts
  4. Your PR introduces the PROJECT_ID environemnt variable in the auth-context.tsx file on the dashboard app.
    • Rename this to VITE_PROJECT_ID
    • Add the variable on the .env.local, .env.production and .env.staging with the value "bandada".
    • use import.meta.env.VITE_PROJECT_ID! instead of process.env.PROJECT_ID!
  5. The auth_service.ts file in the api app use siweMessage.validate(). This is deprecated, you can change it with siweMessage.verify() and change the expected output accordingly.

Let me know if you have any more comments / errors / thoughts on this. Happy to chat and gl with the remaining todos (should be quick!).

@neel-ds
Copy link
Contributor Author

neel-ds commented Mar 28, 2024

Thanks @0xjei, I will do the needful promptly.

@neel-ds neel-ds force-pushed the refactor/ethers-v6 branch from c8fe967 to 7fbebea Compare March 28, 2024 20:09
@neel-ds
Copy link
Contributor Author

neel-ds commented Mar 28, 2024

Thank you very much @0xjei for continuous support. I have completed the requested changes and rebased the branch with dev branch.

@neel-ds neel-ds requested a review from 0xjei March 28, 2024 20:24
@0xjei
Copy link
Contributor

0xjei commented Mar 28, 2024 •
edited
Loading

As we have discussed

Thank you very much @0xjei for continuous support. I have completed the requested changes and rebased the branch with dev branch.

Thank you so much for your effort @neel-ds. Could you please run yarn prettier:write && yarn lint --fix from the root of the project? Also, check if the tests (yarn test) are passing since CI is failing! Thank you again and awesome job here ✨

@neel-ds neel-ds force-pushed the refactor/ethers-v6 branch from cf69c56 to 8e2851b Compare March 29, 2024 16:39
0xjei
0xjei suggested changes Apr 5, 2024
View reviewed changes
Copy link
Contributor

@0xjei 0xjei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great @neel-ds, thanks again for the extraordinary progress! Only the last 2 steps are missing eheh

  • For the failed test on auth.service (Should throw an error if the signature is invalid) you can delete this test case completely. After some research, the test is wrong and not needed.
  • Use bandada.target instead of bandada.address as mentioned in the other comment.

Looking forward to your changes 😎

apps/contracts/test/BandadaSemaphore.ts Show resolved Hide resolved
@neel-ds neel-ds force-pushed the refactor/ethers-v6 branch from 8e2851b to bb219b1 Compare April 6, 2024 06:49
0xjei
0xjei approved these changes Apr 9, 2024
View reviewed changes
Copy link
Contributor

@0xjei 0xjei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing work @neel-ds!!! Thank you so much for your effort, patience and work! Looking forward to collab again with you :)

@neel-ds neel-ds force-pushed the refactor/ethers-v6 branch from ec3ac65 to 6f0316e Compare April 9, 2024 17:58
neel-ds added 6 commits April 11, 2024 11:47
@neel-ds
refactor: upgrade ethers in apps
84170c6
@neel-ds
refactor: upgrade ethers dependency in libs
256d749 
fix: resolve rebase conflicts

fix: resolve rebase conflicts

fix: resolve rebase conflicts

fix: resolve rebase conficts

fix: resolve rebase conflicts
@neel-ds
refactor: update wagmi dependency
5cb7672 
fix: resolve rebase conflicts

fix: resolve rebase conflicts
@neel-ds
refactor: upgrade typescript
0d770a0
@neel-ds
fix: union type error
27f1852
@neel-ds
fix: resolve lint error
b49473a
@neel-ds neel-ds force-pushed the refactor/ethers-v6 branch from d5b50e3 to a659eee Compare April 11, 2024 06:24
@0xjei 0xjei changed the base branch from dev to deps/update April 12, 2024 14:26
@0xjei 0xjei merged commit 9f85cf1 into bandada-infra:deps/update Apr 12, 2024
4 checks passed
@0xjei 0xjei mentioned this pull request May 9, 2024
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@0xjei 0xjei 0xjei approved these changes

@vplasencia vplasencia Awaiting requested review from vplasencia

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@neel-ds @0xjei